Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React Native Implement Basic text Toolbar actions #9267

Merged
merged 7 commits into from Aug 24, 2018

Conversation

SergioEstevao
Copy link
Contributor

Description

Refs: wordpress-mobile/gutenberg-mobile#128

This PR implements the connection of the toolbar with the richtext component in React Native.

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

return accFormats;
}, {} );

this.setState( { newFormats, selectedNodeId: this.state.selectedNodeId + 1 } );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder why it's needed selectedNodeId: this.state.selectedNodeId + 1 here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy pasted from the GB web... @gziolo you know why is this?

@@ -21,12 +21,20 @@ import { children } from '@wordpress/blocks';
import FormatToolbar from './format-toolbar';
import { FORMATTING_CONTROLS } from './formatting-controls';

export function getFormatValue( formatName ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gziolo I wonder why we need to export this function here. Can 't it be moved in RichText component since it's only used there?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is exported only to be used in tests

Copy link
Contributor

@daniloercoli daniloercoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good Sergio!
There are some problems on Android but I think those are on react-native-aztec only. We will get it sorted in another PR.

@gziolo
Copy link
Member

gziolo commented Aug 24, 2018

Looks good, let's iterate on RichText in future PRs to find a common API for both Aztec and TinyMCE - the more code you add - the easier it's going to be to reason about those two implementations.

@gziolo gziolo merged commit 7f0e824 into master Aug 24, 2018
@gziolo gziolo deleted the rnmobile/toolbar_actions branch August 24, 2018 05:24
@gziolo gziolo added this to the 3.7 milestone Aug 24, 2018
@gziolo gziolo added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants